Skip to content

Fix misleading slug display in stardag modal CLI (v0.7.3)#143

Merged
andhus merged 2 commits into
mainfrom
fix-modal-cli-slug-display
May 8, 2026
Merged

Fix misleading slug display in stardag modal CLI (v0.7.3)#143
andhus merged 2 commits into
mainfrom
fix-modal-cli-slug-display

Conversation

@andhus
Copy link
Copy Markdown
Collaborator

@andhus andhus commented May 8, 2026 •
edited
Loading

Summary

stardag modal deploy and stardag modal stardag-api-key create printed the workspace/environment slug by reading the active CLI profile's TOML (prof["environment"]). When env vars or a custom config_provider default-factory override the resolved IDs, the slug pulled from the active profile may not correspond to the resolved UUID — producing hypothetical output like:

Environment: <env-a-uuid> (env-b-slug)

…where the UUID resolves to environment A but the slug is read from a profile pointing at environment B. Deployed bytes were always correct; only the printed line was misleading.

Fix

  • Added get_cached_workspace_slug / get_cached_environment_slug reverse lookups in stardag/config/cache.py (UUID → slug, against ~/.stardag/id-cache.json).
  • Replaced _get_profile_slugs with _resolve_display_slugs in stardag/_cli/modal.py. The slug shown is now always the one that actually maps to the resolved UUID, or omitted when there is no cache entry — instead of guessing from the active profile.

Includes minimal CHANGELOG.md + RELEASE_NOTES.md entries for v0.7.3.

Test plan

  • pre-commit run passes (ruff, pyright, prettier)
  • Reverse lookup verified against a populated id-cache (UUID resolves to its actual slug, not the active profile's slug)
  • Local smoke: stardag modal deploy ... from a project that overrides STARDAG_WORKSPACE_ID / STARDAG_ENVIRONMENT_ID via a custom config_provider default factory now prints the slug matching the override, not the active profile's slug

🤖 Generated with Claude Code

@andhus andhus requested a review from Copilot May 8, 2026 10:21
@andhus @claude
Fix misleading slug display in stardag modal CLI (v0.7.3)
1f19a92 
The workspace/environment slug printed by `stardag modal deploy` and
`stardag modal stardag-api-key create` was read from the active CLI
profile's TOML, so when env vars or a custom `config_provider` override
the resolved IDs the slug could refer to a different env than the
resolved UUID — e.g. hypothetically `Environment: <env-a-uuid>
(env-b-slug)` where the UUID resolves to env A but the slug is read
from a profile pointing at env B.

Replace `_get_profile_slugs` with `_resolve_display_slugs` which
reverse-looks up the slug from the resolved UUID via the id-cache,
falling back to omitting the slug when there is no cache hit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@andhus andhus force-pushed the fix-modal-cli-slug-display branch from 633e9b4 to 1f19a92 Compare May 8, 2026 10:23
Copilot AI reviewed May 8, 2026
View reviewed changes
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes misleading slug output in the stardag modal CLI by deriving the displayed workspace/environment slugs from the resolved UUIDs (via the local id-cache) rather than from the active CLI profile TOML, and documents the change for release v0.7.3.

Changes:

  • Added UUID → slug reverse-lookup helpers to stardag/config/cache.py using ~/.stardag/id-cache.json.
  • Updated stardag/_cli/modal.py to use reverse-lookups for display (_resolve_display_slugs) instead of reading profile TOML slugs.
  • Added v0.7.3 entries to CHANGELOG.md and RELEASE_NOTES.md.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
RELEASE_NOTES.md Documents the CLI display fix for v0.7.3.
lib/stardag/src/stardag/config/cache.py Adds cached reverse lookups (UUID → slug) for workspace/environment.
lib/stardag/src/stardag/_cli/modal.py Switches slug display logic to use id-cache reverse lookup.
CHANGELOG.md Adds a v0.7.3 changelog entry describing the CLI display fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/stardag/src/stardag/_cli/modal.py Outdated
Comment thread lib/stardag/src/stardag/_cli/modal.py Outdated
@andhus @claude
Pin id-cache lookup to the resolved registry URL, single-load
df7d933 
Address review feedback on PR #143:

1. `_resolve_display_slugs` previously read `registry_name` from
   `get_config()`, but both call sites resolve IDs under a temporarily
   overridden `STARDAG_PROFILE` and restore the env before printing —
   so `get_config()` could point at a different registry than the IDs
   came from, producing reverse-lookup misses on multi-registry setups.
   Derive `registry_name` from the actual `STARDAG_API_URL` instead.

2. Collapse `get_cached_workspace_slug` + `get_cached_environment_slug`
   into a single `get_cached_slugs` helper so the id-cache file is
   loaded once per CLI invocation instead of twice.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@andhus
Copy link
Copy Markdown
Collaborator Author

andhus commented May 8, 2026 •
edited
Loading

Thanks for the review — both comments addressed in df7d933.

1. Registry-name mismatch (real bug, fixed)

You're right that the original _resolve_display_slugs had a residual inconsistency: both deploy(--profile foo) and stardag_api_key_create(--stardag-profile foo) resolve IDs under a temporarily-overridden STARDAG_PROFILE, then restore the env (and clear the config cache) before the display call. So get_config().context.registry_name would point at the active profile rather than foo — and on a multi-registry setup the id-cache reverse lookup would either miss or hit the wrong registry's slugs.

Fixed by pinning the registry to the URL the IDs were actually resolved against:

  • New _registry_name_for_url(api_url) helper that reverse-looks up the registry name from list_registries() (the TOML [registry.<name>] table).
  • _resolve_display_slugs now takes api_url and derives registry_name from it. Callers pass env_vars["STARDAG_API_URL"] (deploy) or api_url from _resolve_stardag_context (api-key create) — both of which were captured while the override was active.
  • Falls back to (None, None) (no slug shown) when the URL has no matching TOML registry, instead of silently using the wrong one.

2. Double id-cache load (minor, fixed)

Collapsed get_cached_workspace_slug + get_cached_environment_slug into a single get_cached_slugs(registry, ws_id, env_id) -> tuple[str | None, str | None] so the cache file is loaded once per invocation.

Verified

Reverse lookup against a populated id-cache returns the slug that actually maps to the resolved UUID. Unknown UUIDs and URLs that don't match any configured TOML registry both fall back to no slug, instead of silently displaying a wrong one.

🤖 Generated with Claude Code

@andhus andhus merged commit 3cdb912 into main May 8, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@andhus